Skip to content

[FIP-29] Add SSL config parsing and SslContext/SslHandler factory - #3813

Open
MicheleGuerriero wants to merge 1 commit into
apache:mainfrom
MicheleGuerriero:3796-ssl-config-and-sslcontext-factory
Open

[FIP-29] Add SSL config parsing and SslContext/SslHandler factory#3813
MicheleGuerriero wants to merge 1 commit into
apache:mainfrom
MicheleGuerriero:3796-ssl-config-and-sslcontext-factory

Conversation

@MicheleGuerriero

Copy link
Copy Markdown

Purpose

Linked issue: close #3796

First building block of FIP-29 (mTLS support for Fluss RPC). This adds
the configuration surface and a factory for building Netty SSL
contexts/handlers, but does not wire them into any pipeline yet — no
runtime behavior changes as a result of this PR.

Brief change log

  • ConfigOptions: add server-side security.ssl.* options (enabled
    listeners, protocols, cipher suites, keystore/truststore paths and
    passwords, reload interval) and client-side client.security.ssl.*
    options (mirrors the server options, plus endpoint identification
    algorithm for hostname verification).
  • SslConfig: new immutable holder that parses and validates the
    above options from a Configuration, with fromServerConfig/
    fromClientConfig factory methods. Fails fast with a clear message
    if a server enables TLS without configuring a keystore.
  • SslContextFactory: new factory building Netty SslContext/
    SslHandler instances from an SslConfig, using the JDK SSL
    provider. Supports per-listener client-certificate requirement
    (setNeedClientAuth, for mTLS listeners) on the server side, and
    SNI + hostname verification on the client side.

Later tickets (#3792 server pipeline, #3797 client pipeline) will wire
SslContextFactory into the actual Netty channel pipelines.

Tests

Added SslContextFactoryTest, backed by TestSslUtils (generates a
self-signed certificate and on-the-fly JKS keystore/truststore files,
no committed key material):

  • server/client SslContext creation
  • enabled-protocol and cipher-suite filtering
  • key-password fallback to keystore password
  • client endpoint-identification-algorithm toggling (hostname
    verification on/off)
  • server needClientAuth toggling for mTLS
  • an end-to-end embedded-channel TLS handshake between a server and
    client SslHandler, asserting the negotiated session uses a real
    TLS protocol/cipher (not plaintext)
  • fail-fast validation when no keystore is configured

Verified locally on fluss-common and fluss-rpc: mvn test (all
green, SslContextFactoryTest 9/9), spotless:check,
checkstyle:check, Apache RAT license check, and a JDK 8 build
(-Pjava8) confirming Java 8 source compatibility.

API and Format

Adds new ConfigOptions (security.ssl.*, client.security.ssl.*)
and two new @Internal classes in fluss-rpc
(org.apache.fluss.rpc.netty.ssl). No changes to existing public
APIs, RPC wire format, or storage format. The new classes are
currently unused by any production code path.

Documentation

No user-facing behavior yet, so no documentation changes in this PR.
Configuration docs for security.ssl.* / client.security.ssl.*
will be added once these options take effect (server/client pipeline
wiring in the following tickets).

  • No generative AI tools used
  • Yes (Claude Code, Claude Opus 5)

First building block of FIP-29 (mTLS support), see apache#3796.

Adds the server-side security.ssl.* and client-side client.security.ssl.*
config options (protocols, cipher suites, keystore/truststore, endpoint
identification, reload interval), an immutable SslConfig holder parsing
them with fail-fast validation, and an SslContextFactory building Netty
SslContext/SslHandler instances for server and client (JDK provider,
per-listener client-auth requirement, SNI + hostname verification).

These are currently-unused building blocks; later tickets wire them into
the Netty pipeline. No runtime behavior change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@affo affo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The work looks very good and aligned with the FIP 1:1, thank you! 🤝

I only have a couple doubts marked as a comments 🤝

// Server-side TLS (transport encryption + mTLS) options
// ------------------------------------------------------------------------

public static final ConfigOption<List<String>> SERVER_SSL_ENABLED_LISTENERS =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a way to mark these options as "experimental" for the time being 🤔

My concern is that these options are now exposed to the outer world without anything wired, and this may compromise the release (say that we don't finalize the feature and it is half baked).

I would need the feedback of a committer here, or a PMC even :) @fresh-borzoni would you feel like it?

}

/** Build and validate the server-side TLS configuration. */
public static SslConfig fromServerConfig(Configuration conf) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do understand that this contribution does not switch yet on SERVER_SSL_ENABLED_LISTENERS to enforce the validation that remains implicit, but this also makes sense as we don't have yet the wiring and those methods stay not invoked for now.

In the future one should extract the SSL config by checking that.

But that would turn to be cumbersome as part of the logic of checking whether SSL is enabled would fall out of the config class itself.

I wonder if it would be a neater approach to embed right now all this knowledge into this class, may return an Optional to embed the semantics of the fact that SSL may be there or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TLS] Add SSL config parsing and SslContext/SslHandler factory

2 participants